Rework alert/confirm to be tab-modal (instead of application-modal) #7107

bsclifton · 2017-02-06T17:24:33Z

Test Plan

Testing alert

Visit http://jsbin.com/fiyojusahu/edit?html,output
In the output area, click the Click me to test an alert button
Confirm the alert works as expected

Testing confirm

Visit http://jsbin.com/sadunogefu/edit?html,output
In the output area, click the Click me to test a confirm button
Confirm the confirm works as expected (try clicking OK and also cancel)

Testing import

Have a bookmarks file (.html) or other browser history which can be imported
Launch Brave and go to Preferences > General
Click import, choose appropriate settings (based on what you have in step 1)
Click OK to start import process
When done, you should see a tab-specific modal letting you know it's complete

Testing import (Firefox)

Have Firefox installed and open
Launch Brave and go to Preferences > General
Click import, choose Firefox
Click OK to start import process
You should see warning alert

Issue specific tests

#3794 - Hard to exit/close Brave when site spams you with message box / alerts
#2755 - prevent js alert spoofing attacks
#6901 - alert() popups should appear below tabs
#7213 - Brave crashed on macOS 10.12.3 via.window.open()
#7280 - Invalid sync code alert needs proper heading

Automated tests

# unit tests
npm run unittest -- --grep="MessageBox component unit tests"
npm run unittest -- --grep="tabMessageBoxState unit tests"
npm run unittest -- --grep="tabMessageBox unit tests"
npm run unittest -- --grep="tabState unit tests"
npm run unittest -- --grep="Main component unit tests"
npm run unittest -- --grep="NavigationBar component unit tests"
npm run unittest -- --grep="UrlBar component unit tests"
npm run unittest -- --grep="UrlBarIcon component unit tests"

# webdriver tests
npm run uitest -- --grep="MessageBox component tests"

Description

Submitted a ticket for my issue if one did not already exist.
Used Github auto-closing keywords in the commit message.
Added/updated tests for this change (for new code or code which already has tests).
Ran git rebase -i to squash commits (if needed).

Rework alert/confirm to be tab-modal (instead of application-modal)

Message box:

shows an on/off switch for "Prevent this page from creating additional dialogs". Fixes Hard to exit/close Brave when site spams you with message box / alerts #3794
does not include Brave logo; shows origin; does not need to show "switch to tab" because it's already bound to tab. Fixes prevent js alert spoofing attacks #2755
is shown BELOW tabs now. Fixes alert() popups should appear below tabs #6901

Other changes include:

Removed "An embedded page at " from source message (shown on every message)
Rename showMessageBox/hideMessageBox/clearMessageBoxes actions to showNotification/hideNotification/clearNotification
Converted most styles to Aphrodite

Auditors: @bridiver, @diracdeltas
Folks who may also be interested: @bbondy, @darkdh

Special note

Electron recently received a PR to implement an optional checkbox in electron.dialog.showMessageBox... but this is a better choice for us because it's tab-modal (you can switch tabs while the alert is up, etc).

Screenshots

cc: @ayumi

luixxiul · 2017-02-06T19:10:45Z

Nice work @bsclifton ! Does this PR fix the spoofing dialog like #4992? (I know it's labelled duplicated, I just wanted to make it clear)

diracdeltas · 2017-02-06T19:29:11Z

app/browser/messageBox.js

+
+  close: (state, action) => {
+    state = makeImmutable(state)
+    action = makeImmutable(action)


[minor] setResponse calls makeImmutable on the args, so it's not necessary here

diracdeltas · 2017-02-06T19:33:34Z

app/renderer/components/messageBox.js

+        onClick={this.onDismiss.bind(this, this.tabId, index)} />)
+    }
+
+    return buttons


[minor] could return this.buttons.map(...) instead of iterating through this.buttons

I did originally implement it like that- but the buttons have to be shown in reverse order (in order to keep the same "interface" as the electron dialog.showMessageBox).

ex: buttons would be ['OK', 'Cancel'] with cancel being index 1 (and the caller specifies cancelId === 1)

i think you can do maxIndex - index to get the index reversed

bsclifton · 2017-02-06T19:37:15Z

@luixxiul yes- it does. By removing the logo and also showing the origin, I believe the concern raised in #4992 should be fixed. @diracdeltas can also confirm/deny 😄

diracdeltas · 2017-02-06T19:46:01Z

app/common/state/messageBoxState.js

+const tabState = require('./tabState')
+const {makeImmutable} = require('./immutableUtil')
+
+const messageBoxDetail = 'messageBoxDetail'


does this need to be added to docs/state.md? does it need to be cleaned from tabs on shutdown?

diracdeltas · 2017-02-06T19:51:26Z

app/index.js

+        showSuppress: shouldDisplaySuppressCheckbox
+      }
+
+      MessageBox.show(tabId, detail, cb)


[minor] the enter key should be bound to OK

diracdeltas · 2017-02-06T19:57:59Z

the test plan worked for me. 🎆

however, now that the modal is below the line-of-trust (AKA fully in the webview area), it can be fully spoofed by the page. this is not the case in Chrome/etc. because the modal is its own window, which can be moved outside the webview area; in our case it is immovable.

i see two potential downsides:

this can be abused by the page to make it appear that Brave keeps showing dialogs even though the user has checked the 'suppress' checkbox.
if the alert box is covering something that the user wants to read, they have to close the alert to read it.

bridiver · 2017-02-07T02:59:45Z

accidentally made the comments in your repo @bsclifton, but looks like they still appear above

bsclifton · 2017-02-07T05:05:36Z

Thanks for the feedback, folks! I'm working on tests and am/will be addressing all of the above items 😄

luixxiul · 2017-02-07T05:09:45Z

app/renderer/components/styles/commonStyles.js

+  },
+  'flyoutDialog': {
+    backgroundColor: `${globalStyles.color.toolbarBackground}`,
+    borderRadius: `${globalStyles.radius.borderRadius}`,


Do we need the blackets here? I think this should work: backgroundColor: globalStyles.color.toolbarBackground, and borderRadius: globalStyles.radius.borderRadius,. cf: https://github.com/brave/browser-laptop/pull/7107/files#diff-fa59d1e0221a5bee94fd2abd9b96dcfdR12

luixxiul · 2017-02-07T05:09:52Z

app/renderer/components/styles/commonStyles.js

+    padding: '10px 30px',
+    position: 'absolute',
+    textAlign: 'left',
+    top: `${globalStyles.spacing.dialogTopOffset}`


luixxiul · 2017-02-07T05:15:48Z

app/renderer/components/messageBox.js

+    marginBottom: '1.5em'
+  },
+  'buttons': {
+    float: 'right',


This works enough but I would not set with float to avoid the possibility of the case like this: #6749 (comment). I would instead use display: flex and justify-content: flex-end

luixxiul · 2017-02-07T05:17:51Z

app/renderer/components/styles/commonStyles.js

@@ -19,6 +19,17 @@ const styles = StyleSheet.create({
    outline: 'none',
    padding: '0.4em',
    width: '100%'
+  },
+  'flyoutDialog': {


I'm going to create a tracking issue to delete

browser-laptop/less/forms.less

Lines 8 to 18 in 4fb5d5a

.flyoutDialog {

background-color: @toolbarBackground;

border-radius: @borderRadius;

box-shadow: 2px 2px 8px #3b3b3b;

color: #000;

font-size: 13px;

padding: 10px 30px;

position: absolute;

text-align: left;

top: @dialogTopOffset;

}

in favor of these lines :-)

posted here: #7114

bridiver · 2017-02-07T05:17:57Z

@diracdeltas I'm not sure if we need to address those issues right now. A page can spoof an alert dialog and even if it doesn't resemble what Brave does, a lot of users don't know the difference anyway. The big issue I think is preventing people from navigating away when that happens and this fixes that issue. For the second issue, the old dialog could potentially cover content as well. Maybe file separate issues for them as improvements?

bsclifton · 2017-02-07T08:22:14Z

Round 1 of fixes done w/ 5f2d239

Coming up next:

~~binding enter/escape keys~~
~~webdriver tests enforcing the behaviors called out above in feedback~~

@luixxiul regarding the float: right comment, did you want to push a fix? (my flexbox skills are not the best) I'd love to have a proper style update (and you're welcome to commit one 😄 )

bsclifton · 2017-02-07T08:27:08Z

Also one open I had for you all ( @bridiver, @diracdeltas ):

While writing my tests, I noticed that (because the UI isn't blocked), you can type in the URL bar (including hitting enter) when an alert is up. Nothing happens of course... at least, until you close the alert/confirm.

How should we address this behavior? This could be a chance to address the feedback about pages being able to fake the alert. We can update our alert to disable/gray out the URL bar or any other action we'd like. I do think it's worth keeping the ability to switch tabs though

cc: @bradleyrichter

luixxiul · 2017-02-07T09:15:07Z

regarding the float: right comment, did you want to push a fix?

I will do with a follow-up task!

bradleyrichter · 2017-02-07T15:47:32Z

@bsclifton It would be better to make the URL bar non-editable and perhaps disabled in appearance for this case if you can not actually execute a new search or page change in this state.

diracdeltas · 2017-02-07T19:04:44Z

Maybe file separate issues for them as improvements?

sgtm

bsclifton · 2017-02-23T00:49:35Z

Ready for re-review! cc: @bridiver, @diracdeltas, @bbondy

This PR is mostly tests- it adds 93 tests (81 unit tests, 12 webdriver). Be sure to check all the scenarios tested... they should be covered 😄 If you want to run the tests, you'll find the npm run commands in the top post here ^^

I've also manually tested each scenario. Should be good to go 😄

NejcZdovc · 2017-02-27T21:28:00Z

app/browser/tabMessageBox.js

+      const detail = {
+        message,
+        title,
+        buttons: ['OK'],


I think we should add translated string here

NejcZdovc · 2017-02-27T21:28:08Z

app/browser/tabMessageBox.js

+      const detail = {
+        message,
+        title,
+        buttons: ['OK', 'Cancel'],


I think we should add translated string here

NejcZdovc · 2017-02-27T21:28:32Z

app/renderer/components/messageBox.js

+  }
+
+  get buttons () {
+    return (this.props.detail && this.props.detail.get('buttons')) || makeImmutable(['OK'])


I think we should add translated string here

good catch! It already is being translated (these strings get passed to l10n-Id in render)... however, it's expected (per the app.properties file) to be lowercase. I fixed and verified it works by changing the string to nonsense 😄

Yeah I notice that button component has I10n-Id, but when I changed to the another language there was still the same string.

NejcZdovc · 2017-02-27T21:29:38Z

docs/state.md

+    canGoForward: boolean, // the tab can be navigated forward
+    muted: boolean, // is the tab muted
+    windowId: number, // the windowId that contains the tab
+    messageBoxDetail: { // fields used if showing a message box for a tab


Please order these and child properties alphabetically

NejcZdovc · 2017-02-27T21:31:10Z

app/browser/tabMessageBox.js

+    process.on('window-prompt', (webContents, extraData, title, message, defaultPromptText,
+          shouldDisplaySuppressCheckbox, isBeforeUnloadDialog, isReload, cb) => {
+      console.warn('window.prompt is not supported yet')
+      let suppress = false


NejcZdovc · 2017-02-27T21:31:48Z

app/browser/tabMessageBox.js

+    const tabId = action.getIn(['tabValue', 'tabId'])
+    if (tabId) {
+      // remove callback; call w/ defaults
+      let cb = messageBoxCallbacks[tabId]


NejcZdovc · 2017-02-27T21:32:08Z

app/browser/tabMessageBox.js

+        // remove detail from state
+        state = tabMessageBoxState.removeDetail(state, removeAction)
+        // remove callback; call w/ defaults
+        let cb = messageBoxCallbacks[tabId]


NejcZdovc · 2017-02-27T21:51:22Z

app/renderer/components/messageBox.js

+class MessageBox extends ImmutableComponent {
+  constructor () {
+    super()
+    this.onClick = this.onClick.bind(this)


I think this is not necessary, because you are just stopping propagation in this function

Good call on this- I copied from another modal and it was unnecessary. It may have originally been put there to prevent the modal from closing on click... but that doesn't happen here 👍

NejcZdovc · 2017-02-27T21:55:42Z

app/renderer/components/messageBox.js

+  }
+
+  render () {
+    return <Dialog className='noOutline'>


We should use Aphrodite here as well

Also good catch- I wasn't sure about it at first, because Dialog is not an Aphrodite component... but adding a className using Aphrodite works just fine 👍

NejcZdovc · 2017-02-27T21:56:23Z

less/dialogs.less

@@ -16,6 +16,10 @@
  align-items: center;
  background: rgba(0, 0, 0, 0.15);

+  &.noOutline {


We should use Aphrodite

diracdeltas · 2017-02-28T00:06:30Z

test/unit/app/renderer/components/messageBoxTest.js

+          />
+        )
+        const instance = wrapper.instance()
+        assert.equal(instance.title, 'Brave says:')


diracdeltas · 2017-02-28T00:18:24Z

this seems to make the notificationBar login form tests fail: https://travis-ci.org/brave/browser-laptop/builds/205211560#L5533.

they pass on master with npm run test -- --grep='notificationBar' but the same command also fails locally in this branch

cezaraugusto · 2017-02-28T04:49:14Z

app/renderer/components/messageBox.js

+}
+
+const styles = StyleSheet.create({
+  'container': {


no need to use quotes on keys for Aphrodite

cezaraugusto · 2017-02-28T04:49:38Z

app/renderer/components/messageBox.js

+  },
+  'title': {
+    fontWeight: 'bold',
+    fontSize: '12pt',


what's the reason of using pt instead of px?

Good question- I was thinking it would be nice to use official point sizes and then have everything else relative using em. @NejcZdovc suggested using rem for font size. We should work together and propose a standard cc: @luixxiul

cezaraugusto · 2017-02-28T04:50:19Z

app/renderer/components/messageBox.js

+    fontSize: '12pt',
+    marginBottom: '1em',
+    marginTop: '0.5em',
+    '-webkit-user-select': 'text'


would be better using WebkitUserSelect instead of stringifying

@cezaraugusto does this work even for proprietary extensions? (vendor prefix; starting with a -)?

yes, you can double-check looking at devTools for resulted code. I guess Aphrodite uses React built-in styles vendor mechanism. For reference:

https://facebook.github.io/react/docs/dom-elements.html#style

cezaraugusto · 2017-02-28T04:50:33Z

app/renderer/components/messageBox.js

+    marginTop: '1.5em',
+    minWidth: '425px',
+    marginBottom: '1.5em',
+    '-webkit-user-select': 'text'


cezaraugusto · 2017-02-28T04:51:49Z

app/renderer/components/urlBarIcon.js

-      onDragStart={this.onDragStart}
-      draggable
-      onClick={this.onClick}
+      {...props}


++ spread operator ftw

NejcZdovc · 2017-02-28T07:23:20Z

docs/state.md

@@ -279,16 +279,16 @@ AppStore
    audible: boolean, // is audio playing (muted or not)
    canGoBack: boolean, // the tab can be navigated back
    canGoForward: boolean, // the tab can be navigated forward
-    muted: boolean, // is the tab muted
-    windowId: number, // the windowId that contains the tab
    messageBoxDetail: { // fields used if showing a message box for a tab
      message: string,


These properties should be alphabetize as well

There is a comment showing some which are session and some which are persistent. We can alphabetize but then remove that label, but I don't see value with that

tabs: [{ // persistent properties active: boolean, // whether the tab is selected favIconUrl: string, id: number, index: number, // the position of the tab in the window title: string, url: string, windowUUID: string, // the permanent identifier for the window // session properties audible: boolean, // is audio playing (muted or not) canGoBack: boolean, // the tab can be navigated back canGoForward: boolean, // the tab can be navigated forward messageBoxDetail: { // fields used if showing a message box for a tab message: string, title: string, // title is the source; ex: "brave.com says:" buttons: [string], // array of buttons as string; code only handles 1 or 2 suppress: boolean, // if true, show a suppress checkbox (defaulted to not checked) showSuppress: boolean, // final result of the suppress checkbox cancelId: number // optional: used for a confirm message box }, muted: boolean, // is the tab muted windowId: number // the windowId that contains the tab }],

I am talking about these properties:

messageBoxDetail: { // fields used if showing a message box for a tab message: string, title: string, // title is the source; ex: "brave.com says:" buttons: [string], // array of buttons as string; code only handles 1 or 2 suppress: boolean, // if true, show a suppress checkbox (defaulted to not checked) showSuppress: boolean, // final result of the suppress checkbox cancelId: number // optional: used for a confirm message box }

doh my bad- fixed 😄

Message box: - shows an on/off switch for "Prevent this page from creating additional dialogs". Fixes #3794 - does not include Brave logo; shows origin; does not need to show "switch to tab" because it's already bound to tab. Fixes #2755 - is shown BELOW tabs now. Fixes #6901 - Fixes #7213 Other changes include: - Removed "An embedded page at " from source message (shown on every message) - Rename showMessageBox/hideMessageBox/clearMessageBoxes actions to showNotification/hideNotification/clearNotification - Converted most styles to Aphrodite When active tab has an alert: - URL bar puts itself in title mode - disable / gray out lion icon - back/forward buttons are disabled - URL bar icon (lock, etc) is no longer clickable/draggable Includes first round of fixes (post-feedback) - renamed actions - unit tests for messageBoxState - component tests for MessageBox - update to state.md - MessageBox control responds properly to keyboard (escape / enter) - webdriver tests for alert/confirm TODO / WIP: - call callback if navigating away (click link, close tab, etc) - tests which ensure callback is cleaned up on crash/close of tab

@bbondy

- Added webdriver tests for opening new tab / navigating when alert is open - created tests for the `app/browser/messageBox` code - created new tabMessageBoxReducer - renamed msgbox actions to be declarative - updated onDestroy/onCrashed to call callback and remove callback - added test to ensure on() handler is registered for webcontents - Moved `process.on` handlers for message box into the new reducer (`window-alert`, `window-confirm`, `window-prompt`) Includes some renaming to make things consistent Auditors: @bbondy, @bridiver

- Main.js component now properly disables shields button and hides extensions when active tab is showing an alert/confirm. - Message box for tab is now cleared (callback called + removed) when tab showing alert navigates - Removed destroy/crash handlers for webcontents in favor of APP_TAB_CLOSED action - Removed session test; replaced with a proper tab-specific session test

@diracdeltas

Fixes #7280 Auditors: @diracdeltas

@NejcZdovc

- "ok"/"cancel" are now localized - tests for notificationBar work great now (miss on my part after rebase) - aphrodite the dialog / drop the less change - const over let - no quotes on keys - alphabetize state.md Auditors: @NejcZdovc, @diracdeltas, @cezaraugusto

NejcZdovc · 2017-02-28T07:37:11Z

We should change this two as well:

@darkdh

Auditors: @darkdh, @NejcZdovc I was easily able to test the happy path (showImportSuccess), but I am not sure how to test the other (showImportWarning). Also (per the comments in place), I don't anticipate any issues with the menu, but would like your input @darkdh :)

bsclifton · 2017-02-28T08:26:21Z

@NejcZdovc yes- great catch 😄 I went ahead and updated the import code to also use this alert.

bbondy · 2017-02-28T15:03:18Z

I assume there will be some follow ups but this is looking very good, great work on this. I'll merge now.

Closes #7949 Also modify messageBox.js, which was introduced with #7107 - Introduced globalStyles.spacing.dialogInsideMargin As the marginTop of title and marginBottom of buttons on messageBox.js had been set to 0.5em, I removed and added them to padding of flyoutDialog. The vaule was calculated this way: calc(10px + 0.5rem) = 18px. Also I applied it to the elements inside messageBox.js - Aligned the buttons to the right (same as messageBox.js) - Introduced siteInfo__wrapperLarge As the buttons on mixed content info dialog are so huge that sometimes they are wrapped, epecially on foreign language like Japanese. This is a temporary workaround to avoid it. - Moved 'denyRunInsecureContent' button to the right to improve UX - Updated automated test code Auditors: Test Plan: 1. Open https://apple.com 2. Click the lock icon on the URL bar 3. Make sure the title and description are aligned 4. Make sure the button is aligned to the right 5. Open http://apple.com 6. Click the unlock icon on the URL bar 7. Make sure the title and description are aligned 8. Open https://mixed-script.badssl.com 9. Click the lock icon 10. Make sure the buttons are aligned to the right and they are not wrapped 11. Click "Load Unsafe Script" 12. Click the unlock icon on the URL bar 13. Click "Stop Loading Unsafe Script" 14. Visit http://jsbin.com/fiyojusahu/edit?html,output 15. Make sure the elements are aligned well 16. Visit http://jsbin.com/sadunogefu/edit?html,output 17. Make sure the switch next to "Prevent this page from creating additional dialogs" is aligned with the body text

bsclifton added accessibility security labels Feb 6, 2017

bsclifton self-assigned this Feb 6, 2017

bsclifton requested review from bridiver and diracdeltas February 6, 2017 17:24

bsclifton added this to the 0.13.3 milestone Feb 6, 2017

bsclifton added the PR/needs-QA-attention ☕ label Feb 6, 2017

diracdeltas reviewed Feb 6, 2017

View reviewed changes

luixxiul added the PR/needs-rebase ۞ label Feb 7, 2017

luixxiul reviewed Feb 7, 2017

View reviewed changes

luixxiul mentioned this pull request Feb 7, 2017

Refactor elements styled with .flyoutDialog class with Aphrodite #7114

Closed

bsclifton removed the PR/needs-rebase ۞ label Feb 7, 2017

bsclifton mentioned this pull request Feb 7, 2017

"modal" dialogs for tabs aren't really modal #3574

Closed

bsclifton requested a review from NejcZdovc February 27, 2017 21:19

NejcZdovc reviewed Feb 27, 2017

View reviewed changes

diracdeltas reviewed Feb 28, 2017

View reviewed changes

cezaraugusto reviewed Feb 28, 2017

View reviewed changes

bsclifton requested a review from cezaraugusto February 28, 2017 05:41

NejcZdovc reviewed Feb 28, 2017

View reviewed changes

bsclifton added 5 commits February 28, 2017 00:35

Update MessageBox component to substitute "Brave" for the extension ID

9118cf5

Fixes #7280 Auditors: @diracdeltas

bbondy merged commit 67b7687 into brave:master Feb 28, 2017

bsclifton deleted the prevent-alert-spam branch February 28, 2017 15:56

luixxiul removed the PR/needs-QA-attention ☕ label Feb 28, 2017

ayumi mentioned this pull request Feb 28, 2017

alert() popups should appear below tabs #6901

Closed

This was referenced Mar 13, 2017

View certificate on padlock #7319

Merged

Fixed UI on alert dialogs (Replace float:right with flex) #7673

Closed

Replace float:right with flex model on alert dialogs #7674

Merged

This was referenced Mar 28, 2017

Alert box with quite lengthy content cannot be closed #7930

Closed

Refactor siteInfo.js with Aphrodite #7985

Merged

bbondy mentioned this pull request May 30, 2017

Make autofill be tab modal and not window modal #4266

Closed

luixxiul mentioned this pull request May 30, 2017

JS Windows should be tab modal, not application modal #1990

Closed

bsclifton mentioned this pull request Jun 22, 2017

Alert disables UI elements and hides URL bar #9285

Closed

	.flyoutDialog {
	background-color: @toolbarBackground;
	border-radius: @borderRadius;
	box-shadow: 2px 2px 8px #3b3b3b;
	color: #000;
	font-size: 13px;
	padding: 10px 30px;
	position: absolute;
	text-align: left;
	top: @dialogTopOffset;
	}

Rework alert/confirm to be tab-modal (instead of application-modal) #7107

Rework alert/confirm to be tab-modal (instead of application-modal) #7107

Conversation

bsclifton commented Feb 6, 2017 • edited Loading

Test Plan

Testing alert

Testing confirm

Testing import

Testing import (Firefox)

Issue specific tests

Automated tests

Description

Special note

Screenshots

luixxiul commented Feb 6, 2017

diracdeltas Feb 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsclifton commented Feb 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diracdeltas commented Feb 6, 2017

bridiver commented Feb 7, 2017

bsclifton commented Feb 7, 2017

luixxiul Feb 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridiver commented Feb 7, 2017

bsclifton commented Feb 7, 2017 • edited Loading

bsclifton commented Feb 7, 2017

luixxiul commented Feb 7, 2017

bradleyrichter commented Feb 7, 2017

diracdeltas commented Feb 7, 2017

bsclifton commented Feb 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diracdeltas commented Feb 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsclifton Feb 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NejcZdovc commented Feb 28, 2017 • edited Loading

bsclifton commented Feb 28, 2017

bbondy commented Feb 28, 2017

bsclifton commented Feb 6, 2017 •

edited

Loading

diracdeltas Feb 6, 2017 •

edited

Loading

bsclifton commented Feb 6, 2017 •

edited

Loading

luixxiul Feb 7, 2017 •

edited

Loading

bsclifton commented Feb 7, 2017 •

edited

Loading

bsclifton Feb 28, 2017 •

edited

Loading

NejcZdovc commented Feb 28, 2017 •

edited

Loading